Add UDP support to NetX sockets for DTLS sessions#10408
Conversation
|
Can one of the admins verify this patch? |
|
Okay to test. Contributor agreement on file. Thank you @hmohide |
|
#10414 Should fix the zephyr tests if you rebase to pull in the updated workflow |
|
|
@dgarskeI think above failure not triggered by the changes? Do i need to make some change? Please guide |
|
Hi @hmohide these would all be solved by a rebase against latest master and force push. Please go ahead and do that. Thanks |
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 9 total — 9 posted, 0 skipped
7 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [High] DTLS NetX receive maps timeouts to fatal error, breaking retransmission —
src/wolfio.c:2659-2665 - [Medium] wolfSSL_SetIO_NetX_Dtls stores caller pointers dereferenced much later (lifetime hazard) —
src/wolfio.c:2751-2759, wolfssl/internal.h:5719-5721 - [Medium] DTLS receive does not capture peer address (single fixed-peer limitation) —
src/wolfio.c:2644-2694 - [Medium] New public API and DTLS IO callbacks have no test coverage —
src/wolfio.c:2644-2759 - [Medium] New public API lacks doxygen; existing doxygen references renamed nxSocket field —
wolfssl/wolfio.h:798-801 - [Medium] DTLS NetX path requires NetX Duo APIs without a guard —
src/wolfio.c:2729-2745 - [Low] NetX_SendTo uses Allman braces and missing space after 'if' (style mismatch) —
src/wolfio.c:2729-2745 - [Low] Trailing whitespace will fail wolfSSL whitespace CI check —
src/wolfio.c:2640,2708,2736,2744,2754; wolfssl/internal.h:5718; wolfssl/wolfio.h:795,798-801 - [Low] Incorrect/misleading struct member comments (nxPort, nxUdpSocket) —
wolfssl/internal.h:5719-5721
Review generated by Skoll
dgarske
left a comment
There was a problem hiding this comment.
@hmohide Our internal AI review found several items to consider: #10408 (review)
|
@dgarske i have updated the review comments |
- Replace non-ASCII em-dash with ASCII hyphen in NetX_ReceiveFrom comment to pass wolfSSL encoding/whitespace CI check - Add WOLFSSL_NETX_DUO to .wolfssl_known_macro_extras so the macro checker no longer flags it as unrecognized
|
Jenkins retest this please "Build 'wolfSSL/PRB-fips-repo-and-harness-test-v3-part2' failed with result: FAILURE" |
There was a problem hiding this comment.
Pull request overview
This PR adds NetX Duo UDP I/O callback support so DTLS sessions can run over NetX sockets (in addition to existing NetX TCP/TLS support), and wires the appropriate callbacks/context initialization into the default wolfSSL initialization paths.
Changes:
- Add NetX Duo DTLS UDP I/O callbacks (
NetX_ReceiveFrom/NetX_SendTo) and a public configuration API (wolfSSL_SetIO_NetX_Dtls). - Update NetX context structure to track separate TCP vs UDP socket handles and UDP destination addressing.
- Select UDP callbacks automatically for DTLS methods in
InitSSL_Ctx()and ensure DTLS I/O contexts point atssl->nxCtxwhen NetX is enabled.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/wolfio.h |
Adds public API and local callback prototypes for NetX Duo DTLS UDP I/O. |
wolfssl/internal.h |
Extends NetX I/O context to distinguish TCP vs UDP and store DTLS destination addressing. |
src/wolfio.c |
Implements NetX Duo DTLS UDP send/receive callbacks and updates existing NetX TCP callbacks to use renamed context fields. |
src/internal.c |
Selects NetX DTLS UDP callbacks for DTLS contexts and fixes DTLS default I/O ctx initialization for NetX. |
doc/dox_comments/header_files/wolfio.h |
Updates NetX docs and adds Doxygen for the new DTLS NetX Duo UDP API. |
doc/dox_comments/header_files-ja/wolfio.h |
Japanese doc updates/additions corresponding to the NetX changes. |
.wolfssl_known_macro_extras |
Registers WOLFSSL_NETX_DUO as a known macro. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 5 total — 4 posted, 1 skipped
4 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] NetX_ReceiveFrom does not validate the datagram source peer —
src/wolfio.c:2645-2718 - [Medium] DTLS 1.3 quick-timeout can divide to zero and fall back to nxWait, disabling retransmission —
src/wolfio.c:2662-2675 - [Low] Zero-length datagram returns 0 instead of being ignored —
src/wolfio.c:2692-2717 - [Low] Redundant repeated wolfSSL_dtls_get_using_nonblock() calls —
src/wolfio.c:2662-2685
Skipped findings
- [Medium]
No test coverage for new NetX DTLS UDP callbacks
Review generated by Skoll
|
@dgarske i have rebased. can you please review and approve |
|
Jenkins retest this please. History lost |
|
The CI "Build 'wolfSSL/PRB-fips-repo-and-harness-test-v3-part2' failed with result: FAILURE" is failing only because you have this PR opened on master. We can accept with that known CI issue. We are in a release cycle, so this won't be merged for a few days. |
|
@dgarske any update on this PR. this one of the critical PR required for our relaese |
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 4 total — 3 posted, 1 skipped
3 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] NetX_ReceiveFrom does not reject negative sz before using it as an unsigned length —
src/wolfio.c:2668-2806 - [Low] Unreachable return at end of NetX_ReceiveFrom (dead code) —
src/wolfio.c:2821-2823 - [Low] Post-handshake invalid-peer/0-length flood is not rate-limited in blocking mode —
src/wolfio.c:2759-2795
Skipped findings
- [Low]
No test coverage for new DTLS NetX Duo I/O callbacks
Review generated by Skoll
Just a few minor items to review here -> #10408 (review) |
|
Okay to test. Thank you @hmohide |
|
Jenkins retest this please "ERROR: Error fetching remote repo 'origin'" |
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 4 total — 3 posted, 1 skipped
3 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] NetX_ReceiveFrom retains leftover datagram bytes, diverging from DTLS datagram semantics —
src/wolfio.c:2809-2832 - [Low] NetX_SendTo silently uses IPv6 path when destination address version is unset —
src/wolfio.c:2870-2878 - [Low] wolfSSL_SetIO_NetX_Dtls does not reset any buffered packet/offset state —
src/wolfio.c:2889-2899
Skipped findings
- [Medium]
No test coverage for new NetX Duo DTLS UDP callbacks
Review generated by Skoll
Always release the full datagram after each read instead of buffering leftover bytes across calls. Matches EmbedReceiveFrom/recvfrom behavior.
|
Jenkins retest this please. |
|
Our PRB CI won't pass the FIPS test because you did this from "master" however I know the changes do not impact FIPS. So I am going to accept as-is. Thank you for being an approved contributor. |
Description
Add UDP support to NetX sockets for DTLS sessions
Fixes zd#
src/internal.c
src/wolfio.c
wolfssl/internal.h
wolfssl/wolfio.h
Testing
Yes, verified the DTLS communication with multiple session
Checklist